bakery: Fix MISRA defects
authorAntonio Nino Diaz <[email protected]>
Wed, 31 Oct 2018 15:55:57 +0000 (15:55 +0000)
committerAntonio Nino Diaz <[email protected]>
Thu, 1 Nov 2018 14:15:39 +0000 (14:15 +0000)
Change-Id: I600bc13522ae977db355b6dc5a1695bce39ec130
Signed-off-by: Antonio Nino Diaz <[email protected]>
include/lib/bakery_lock.h
include/lib/spinlock.h
lib/locks/bakery/bakery_lock_coherent.c
lib/locks/bakery/bakery_lock_normal.c

index c80082e9bbc22e00f7b83af7ff30ea0d3b95f4c9..2d1612e17b43b19caad606481032b43a27ccc6f7 100644 (file)
@@ -4,8 +4,8 @@
  * SPDX-License-Identifier: BSD-3-Clause
  */
 
-#ifndef __BAKERY_LOCK_H__
-#define __BAKERY_LOCK_H__
+#ifndef BAKERY_LOCK_H
+#define BAKERY_LOCK_H
 
 #include <platform_def.h>
 
 
 #ifndef __ASSEMBLY__
 #include <cdefs.h>
+#include <stdbool.h>
 #include <stdint.h>
+#include <utils_def.h>
 
 /*****************************************************************************
- * Internal helper macros used by the bakery lock implementation.
+ * Internal helpers used by the bakery lock implementation.
  ****************************************************************************/
+
 /* Convert a ticket to priority */
-#define PRIORITY(t, pos)       (((t) << 8) | (pos))
+static inline unsigned int bakery_get_priority(unsigned int t, unsigned int pos)
+{
+       return (t << 8) | pos;
+}
+
+#define CHOOSING_TICKET                U(0x1)
+#define CHOSEN_TICKET          U(0x0)
+
+static inline bool bakery_is_choosing(unsigned int info)
+{
+       return (info & 1U) == CHOOSING_TICKET;
+}
+
+static inline unsigned int bakery_ticket_number(unsigned int info)
+{
+       return (info >> 1) & 0x7FFFU;
+}
 
-#define CHOOSING_TICKET                0x1
-#define CHOSEN_TICKET          0x0
+static inline uint16_t make_bakery_data(unsigned int choosing, unsigned int num)
+{
+       unsigned int val = (choosing & 0x1U) | (num << 1);
 
-#define bakery_is_choosing(info)       (info & 0x1)
-#define bakery_ticket_number(info)     ((info >> 1) & 0x7FFF)
-#define make_bakery_data(choosing, number) \
-               (((choosing & 0x1) | (number << 1)) & 0xFFFF)
+       return (uint16_t) val;
+}
 
 /*****************************************************************************
  * External bakery lock interface.
@@ -83,4 +101,4 @@ void bakery_lock_release(bakery_lock_t *bakery);
 
 
 #endif /* __ASSEMBLY__ */
-#endif /* __BAKERY_LOCK_H__ */
+#endif /* BAKERY_LOCK_H */
index 8aec7078958b6a8b1ea8a208463a8b4aeb349753..fcd36e85617196e400bcb9d0e974f18f0662cb3a 100644 (file)
@@ -4,8 +4,8 @@
  * SPDX-License-Identifier: BSD-3-Clause
  */
 
-#ifndef __SPINLOCK_H__
-#define __SPINLOCK_H__
+#ifndef SPINLOCK_H
+#define SPINLOCK_H
 
 #ifndef __ASSEMBLY__
 
@@ -26,4 +26,4 @@ void spin_unlock(spinlock_t *lock);
 
 #endif
 
-#endif /* __SPINLOCK_H__ */
+#endif /* SPINLOCK_H */
index 03277c32f7903d0d86484d398cd699210afeb766..8e86790ab543e2445d9a0c1a9496dfec693d50aa 100644 (file)
@@ -35,9 +35,9 @@
  */
 
 #define assert_bakery_entry_valid(_entry, _bakery) do {        \
-       assert(_bakery);                                        \
-       assert(_entry < BAKERY_LOCK_MAX_CPUS);          \
-} while (0)
+       assert((_bakery) != NULL);                      \
+       assert((_entry) < BAKERY_LOCK_MAX_CPUS);        \
+} while (false)
 
 /* Obtain a ticket for a given CPU */
 static unsigned int bakery_get_ticket(bakery_lock_t *bakery, unsigned int me)
@@ -46,7 +46,7 @@ static unsigned int bakery_get_ticket(bakery_lock_t *bakery, unsigned int me)
        unsigned int they;
 
        /* Prevent recursive acquisition */
-       assert(!bakery_ticket_number(bakery->lock_data[me]));
+       assert(bakery_ticket_number(bakery->lock_data[me]) == 0U);
 
        /*
         * Flag that we're busy getting our ticket. All CPUs are iterated in the
@@ -58,9 +58,9 @@ static unsigned int bakery_get_ticket(bakery_lock_t *bakery, unsigned int me)
         * ticket value. That's OK as the lock is acquired based on the priority
         * value, not the ticket value alone.
         */
-       my_ticket = 0;
+       my_ticket = 0U;
        bakery->lock_data[me] = make_bakery_data(CHOOSING_TICKET, my_ticket);
-       for (they = 0; they < BAKERY_LOCK_MAX_CPUS; they++) {
+       for (they = 0U; they < BAKERY_LOCK_MAX_CPUS; they++) {
                their_ticket = bakery_ticket_number(bakery->lock_data[they]);
                if (their_ticket > my_ticket)
                        my_ticket = their_ticket;
@@ -105,8 +105,8 @@ void bakery_lock_get(bakery_lock_t *bakery)
         * Now that we got our ticket, compute our priority value, then compare
         * with that of others, and proceed to acquire the lock
         */
-       my_prio = PRIORITY(my_ticket, me);
-       for (they = 0; they < BAKERY_LOCK_MAX_CPUS; they++) {
+       my_prio = bakery_get_priority(my_ticket, me);
+       for (they = 0U; they < BAKERY_LOCK_MAX_CPUS; they++) {
                if (me == they)
                        continue;
 
@@ -120,7 +120,8 @@ void bakery_lock_get(bakery_lock_t *bakery)
                 * (valid) ticket value. If they do, compare priorities
                 */
                their_ticket = bakery_ticket_number(their_bakery_data);
-               if (their_ticket && (PRIORITY(their_ticket, they) < my_prio)) {
+               if ((their_ticket != 0U) &&
+                   (bakery_get_priority(their_ticket, they) < my_prio)) {
                        /*
                         * They have higher priority (lower value). Wait for
                         * their ticket value to change (either release the lock
@@ -148,7 +149,7 @@ void bakery_lock_release(bakery_lock_t *bakery)
        unsigned int me = plat_my_core_pos();
 
        assert_bakery_entry_valid(me, bakery);
-       assert(bakery_ticket_number(bakery->lock_data[me]));
+       assert(bakery_ticket_number(bakery->lock_data[me]) != 0U);
 
        /*
         * Ensure that other observers see any stores in the critical section
@@ -156,7 +157,7 @@ void bakery_lock_release(bakery_lock_t *bakery)
         * Then signal other waiting contenders.
         */
        dmbst();
-       bakery->lock_data[me] = 0;
+       bakery->lock_data[me] = 0U;
        dsb();
        sev();
 }
index b947da9c7912e758c5e14ed387421da5293c7e65..beae63c7220cbaf21d15e9cab9f66e92de0b29b7 100644 (file)
@@ -53,21 +53,31 @@ CASSERT((PLAT_PERCPU_BAKERY_LOCK_SIZE & (CACHE_WRITEBACK_GRANULE - 1)) == 0, \
 IMPORT_SYM(uintptr_t, __PERCPU_BAKERY_LOCK_SIZE__, PERCPU_BAKERY_LOCK_SIZE);
 #endif
 
-#define get_bakery_info(_cpu_ix, _lock)        \
-       (bakery_info_t *)((uintptr_t)_lock + _cpu_ix * PERCPU_BAKERY_LOCK_SIZE)
+static inline bakery_lock_t *get_bakery_info(unsigned int cpu_ix,
+                                            bakery_lock_t *lock)
+{
+       return (bakery_info_t *)((uintptr_t)lock +
+                               cpu_ix * PERCPU_BAKERY_LOCK_SIZE);
+}
 
-#define write_cache_op(_addr, _cached) \
-                               do {    \
-                                       (_cached ? dccvac((uintptr_t)_addr) :\
-                                               dcivac((uintptr_t)_addr));\
-                                               dsbish();\
-                               } while (0)
+static inline void write_cache_op(uintptr_t addr, bool cached)
+{
+       if (cached)
+               dccvac(addr);
+       else
+               dcivac(addr);
 
-#define read_cache_op(_addr, _cached)  if (_cached) \
-                                           dccivac((uintptr_t)_addr)
+       dsbish();
+}
+
+static inline void read_cache_op(uintptr_t addr, bool cached)
+{
+       if (cached)
+               dccivac(addr);
+}
 
 /* Helper function to check if the lock is acquired */
-static inline int is_lock_acquired(const bakery_info_t *my_bakery_info,
+static inline bool is_lock_acquired(const bakery_info_t *my_bakery_info,
                                                        int is_cached)
 {
        /*
@@ -78,8 +88,8 @@ static inline int is_lock_acquired(const bakery_info_t *my_bakery_info,
         * operations were not propagated to all the caches in the system.
         * Hence do a `read_cache_op()` prior to read.
         */
-       read_cache_op(my_bakery_info, is_cached);
-       return !!(bakery_ticket_number(my_bakery_info->lock_data));
+       read_cache_op((uintptr_t)my_bakery_info, is_cached);
+       return bakery_ticket_number(my_bakery_info->lock_data) != 0U;
 }
 
 static unsigned int bakery_get_ticket(bakery_lock_t *lock,
@@ -94,7 +104,7 @@ static unsigned int bakery_get_ticket(bakery_lock_t *lock,
         * it is not NULL.
         */
        my_bakery_info = get_bakery_info(me, lock);
-       assert(my_bakery_info);
+       assert(my_bakery_info != NULL);
 
        /* Prevent recursive acquisition.*/
        assert(!is_lock_acquired(my_bakery_info, is_cached));
@@ -103,16 +113,16 @@ static unsigned int bakery_get_ticket(bakery_lock_t *lock,
         * Tell other contenders that we are through the bakery doorway i.e.
         * going to allocate a ticket for this cpu.
         */
-       my_ticket = 0;
+       my_ticket = 0U;
        my_bakery_info->lock_data = make_bakery_data(CHOOSING_TICKET, my_ticket);
 
-       write_cache_op(my_bakery_info, is_cached);
+       write_cache_op((uintptr_t)my_bakery_info, is_cached);
 
        /*
         * Iterate through the bakery information of each contender to allocate
         * the highest ticket number for this cpu.
         */
-       for (they = 0; they < BAKERY_LOCK_MAX_CPUS; they++) {
+       for (they = 0U; they < BAKERY_LOCK_MAX_CPUS; they++) {
                if (me == they)
                        continue;
 
@@ -121,9 +131,9 @@ static unsigned int bakery_get_ticket(bakery_lock_t *lock,
                 * ensure that a stale copy is not read.
                 */
                their_bakery_info = get_bakery_info(they, lock);
-               assert(their_bakery_info);
+               assert(their_bakery_info != NULL);
 
-               read_cache_op(their_bakery_info, is_cached);
+               read_cache_op((uintptr_t)their_bakery_info, is_cached);
 
                /*
                 * Update this cpu's ticket number if a higher ticket number is
@@ -141,7 +151,7 @@ static unsigned int bakery_get_ticket(bakery_lock_t *lock,
        ++my_ticket;
        my_bakery_info->lock_data = make_bakery_data(CHOSEN_TICKET, my_ticket);
 
-       write_cache_op(my_bakery_info, is_cached);
+       write_cache_op((uintptr_t)my_bakery_info, is_cached);
 
        return my_ticket;
 }
@@ -167,8 +177,8 @@ void bakery_lock_get(bakery_lock_t *lock)
         * Now that we got our ticket, compute our priority value, then compare
         * with that of others, and proceed to acquire the lock
         */
-       my_prio = PRIORITY(my_ticket, me);
-       for (they = 0; they < BAKERY_LOCK_MAX_CPUS; they++) {
+       my_prio = bakery_get_priority(my_ticket, me);
+       for (they = 0U; they < BAKERY_LOCK_MAX_CPUS; they++) {
                if (me == they)
                        continue;
 
@@ -177,11 +187,11 @@ void bakery_lock_get(bakery_lock_t *lock)
                 * ensure that a stale copy is not read.
                 */
                their_bakery_info = get_bakery_info(they, lock);
-               assert(their_bakery_info);
+               assert(their_bakery_info != NULL);
 
                /* Wait for the contender to get their ticket */
                do {
-                       read_cache_op(their_bakery_info, is_cached);
+                       read_cache_op((uintptr_t)their_bakery_info, is_cached);
                        their_bakery_data = their_bakery_info->lock_data;
                } while (bakery_is_choosing(their_bakery_data));
 
@@ -190,7 +200,7 @@ void bakery_lock_get(bakery_lock_t *lock)
                 * (valid) ticket value. If they do, compare priorities
                 */
                their_ticket = bakery_ticket_number(their_bakery_data);
-               if (their_ticket && (PRIORITY(their_ticket, they) < my_prio)) {
+               if (their_ticket && (bakery_get_priority(their_ticket, they) < my_prio)) {
                        /*
                         * They have higher priority (lower value). Wait for
                         * their ticket value to change (either release the lock
@@ -199,7 +209,7 @@ void bakery_lock_get(bakery_lock_t *lock)
                         */
                        do {
                                wfe();
-                               read_cache_op(their_bakery_info, is_cached);
+                               read_cache_op((uintptr_t)their_bakery_info, is_cached);
                        } while (their_ticket
                                == bakery_ticket_number(their_bakery_info->lock_data));
                }
@@ -231,7 +241,7 @@ void bakery_lock_release(bakery_lock_t *lock)
         * Then signal other waiting contenders.
         */
        dmbst();
-       my_bakery_info->lock_data = 0;
-       write_cache_op(my_bakery_info, is_cached);
+       my_bakery_info->lock_data = 0U;
+       write_cache_op((uintptr_t)my_bakery_info, is_cached);
        sev();
 }